Skip to content

refactor: don't forward -1 as the diagnosticCode #16495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Dec 10, 2022

Description

This change ensures that if the diagnostic code is set to -1 that we
instead forward None as the code. This will hopefully help any tooling
consuming this to not have to add checks for -1 and filter that code
out, since it's a useless code.

For example I just hit on this for the first time and saw this:

Screenshot 2022-12-10 at 11 48 44

Ideally we don't this showing, but if the code is forwarded to the client via LSP like it is in the image, the client has no way to know if -1 is a valid code or not.

ckipp01 added a commit to ckipp01/metals that referenced this pull request Dec 10, 2022
Since this code means nothing, we should ensure that it's not forwarded
to the client. I did upstream a change for this as well in
scala/scala3#16495, but I think we should
still filter this out now since it is happening.
@prolativ
Copy link
Contributor

The code doesn't really look like it does what it's said to do. Maybe committed wrong files?

@dwijnand dwijnand marked this pull request as draft December 12, 2022 10:28
This change ensures that if the diagnostic code is set to -1 that we
instead forward None as the code. This will hopefully help any tooling
consuming this to not have to add checks for -1 and filter that code
out, since it's a useless code.
@ckipp01 ckipp01 marked this pull request as ready for review December 12, 2022 10:42
@ckipp01
Copy link
Member Author

ckipp01 commented Dec 12, 2022

The code doesn't really look like it does what it's said to do. Maybe committed wrong files?

Ahhh 😱 Sorry, I just used gh cli to create the pr and never actually came here to look to realize I did it incorrectly. I've fixed it, and you should see the correct changes now. Sorry!

@prolativ prolativ enabled auto-merge December 12, 2022 12:58
@prolativ prolativ merged commit da50d11 into scala:main Dec 12, 2022
@ckipp01 ckipp01 deleted the codeForward branch December 12, 2022 13:12
ckipp01 added a commit to scalameta/metals that referenced this pull request Dec 12, 2022
Since this code means nothing, we should ensure that it's not forwarded
to the client. I did upstream a change for this as well in
scala/scala3#16495, but I think we should
still filter this out now since it is happening.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants